:eyes: Use LBTC on wapupay PR review by coelhogonzalo · Pull Request #130 · jan3dev/agentic-aqua

coelhogonzalo · 2026-08-16T12:51:42Z

This PR is meant as feedback for the original PR #122

Each commit addresses a separate issue starting with the most relevant first. I ran Kimi3 and Fable to filter out useless "guards" and checks that are sometimes entertained by LLMs and I filtered out other non-issues myself.

Integration test summary with a real Argentinian alias

QA test — WapuPay review fixes (PR #122 follow-up commits)

Date: 2026-08-16 (UTC)
Branch: use-lbtc-on-wapupay-pr-review
Commits under test: 5d90871, 76254e0, 0178bb1, 57eb6b7, b2d1abf, 3560762
(fixes for findings 1–6 of wapupay-report.md)
Environment: live against WapuPay staging, WAPUPAY_BASE_URL=https://be-stage.wapu.app,
local branch code directly (uv run --directory … python), with order storage pointed at
an isolated QA directory — the real ~/.aqua was not touched.
Method: live integration tests (real network calls to staging), not the unit suite —
the unit tests ran separately per commit. No order was ever paid — tentatives expire on
their own; no real funds moved at any point. Two passes: first with the synthetic alias
aqua.test.stage (the sandbox validates any alias), then a full re-run with the real
alias matriz.menudo.saber (§10).

Result summary

#	Check	Method	Result
1	Public `exchange_rates`, prod + staging	live	✅ PASS
2	Keyed `spending_limit` + `quote` (with and without alias)	live	✅ PASS
3	`create_order` USDT rail — pay_instructions, asset id, base units	live	✅ PASS
4	`create_order` LBTC rail	live	⚠️ BLOCKED upstream (staging 400: rail unavailable); clean error, no orphan record
5	Thin-record recovery (finding 1): delete local record → `fund_order`	live	✅ PASS
6	Reload round-trip of the rebuilt record (`from_dict`)	live	✅ PASS
7	`order_status` refresh + flags on the rebuilt record (finding 3 path)	live	✅ PASS
8	Funding window expiry → terminal status flags	live	⚠️ NOT OBSERVED — staging marks expiry asynchronously; still `FUNDING_ISSUED` 6+ min after the window closed (see §8)
9	Guard paths: rail flip, wrong asset, malformed sats, annotations (findings 2–5)	unit tests	✅ PASS (14/14)
10	Full re-run with a real alias (`matriz.menudo.saber`): quote, USDT order, L-BTC retry, thin-record recovery, reload, status	live	✅ PASS (L-BTC still blocked upstream, same 400)

Evidence

1. Public rates — both hosts reachable through the branch code

https://be-prod.wapu.app  OK {"rates": [{"buy": 1554.72, "pair": "USDT/ARS", "sell": 1621.53}, …
https://be-stage.wapu.app OK {"rates": [{"buy": 1554.72, "pair": "USDT/ARS", "sell": 1621.53}, …

2. Keyed read-only calls

base_url in use: https://be-stage.wapu.app
spending_limit: {'available': 1000.0, 'current_limit': 1000, 'spended': 0.0, 'tier': 0}
quote(no alias): {'exchange_rate': 1554.72, 'fee': 0.13, 'total_amount': 6.56,
                  'usdt_amount': 6.43, 'valid_cbu_alias': True}
quote(alias='aqua.test.stage'): valid_cbu_alias=True

3. USDT order — created, funded, correct instructions

create_order(amount_ars="10000", alias="aqua.test.stage", transfer_type="fiat_transfer", funding_method="USDT") → tentative 1e1df359-b959-4e7f-84ce-3e689aaf542d:

"status": "FUNDING_ISSUED", "funded": true,
"funding_currency": "USDT", "funding_network": "LIQUID",
"total_amount_usdt": "6.56", "total_funding_amount_base_units": 656000000,
"total_amount_sats": null, "funding_amount_sat": null,
"asset_id": "ce091c998b83c78bb71a632313ba3760f1763d9cfcffae02258ffa9865a37bd2",
"pay_instructions": "Send exactly 6.56 USDT (656000000 base units) on Liquid to lq1qq25ex…
                     using lw_send_asset (asset_id=ce091c99…7bd2). This total already
                     includes WapuPay's 0.13 USDT fee — …"

Checks that this proves live:

The amount is quoted as integer base units next to the decimal total.
The asset_id is the USDT policy constant → commit 76254e0's asset-vs-rail check
passed on the happy path (a wrong asset would have raised).
No None, no sats, no L-BTC wording on the USDT rail.

4. L-BTC order — upstream unavailable on staging (not a code failure)

ValueError: WapuPay request failed (400 POST):
  Direct-fiat tentative funding via LBTC on Liquid is not available at the moment.

The error is surfaced verbatim with status and method — no silent fallback.
No orphan record: the create failed at create_tentative, before persistence.
Verified right after: the QA orders directory contained only the USDT order file
(persisted order files: ['1e1df359-….json']).
Consequence: the L-BTC rail could not be exercised end-to-end in this QA run. Its
behavior is covered by the unit tests (§9) and by the PR author's earlier live-stage
verification. Re-run this section when WapuPay re-enables the rail on staging.

5. Thin-record recovery (finding 1) — delete the record, re-fund

The local record was deleted, then fund_order ran against staging with no local state:

local records deleted: True
thin fund_order -> funding_currency: USDT
thin fund_order -> total_funding_amount_base_units: 656000000
thin fund_order -> total_amount_sats: None
thin fund_order -> pay_instructions: Send exactly 6.56 USDT (656000000 base units) …

Wire observation: the staging funding response does echo funding_currency
(raw funding_currency: USDT), so the rebuilt record took the rail from the echo, and
the asset-id inference branch added in 5d90871 did not need to fire live. The inference
branch (rail omitted) and the unknown-rail refusal stay covered by unit tests (§9).
The response also carries both expires_at and funding_expires_at, confirming the
PR #129 rename (expires_at) is wire-compatible.

6. Reload round-trip

reloaded -> funding_currency: USDT | base_units: 656000000 | sats: None

The rebuilt record survives from_dict with the correct rail and amounts.

7. `order_status` on the rebuilt record

status: FUNDING_ISSUED | is_final: False | is_failed: False
funding_currency: USDT | base_units: 656000000
expires_at: 2026-08-16T11:09:55Z | warning: None | last_error: None

The poll path (restructured in 0178bb1) refreshed and persisted the record with a
matching rail echo — no false raise, no warning.

8. Expiry → terminal state (upstream behavior finding)

The order was never paid; its funding window closed at 2026-08-16T11:09:55Z. A
once-per-minute poll ran for 14 minutes and the terminal flip was not observed:

2026-08-16T11:09:41Z status: FUNDING_ISSUED | is_final: False | is_failed: False
2026-08-16T11:14:43Z status: FUNDING_ISSUED | is_final: False | is_failed: False   (poll end)
2026-08-16T11:16:04Z status: FUNDING_ISSUED | is_final: False | is_failed: False   (manual check)

Finding about the upstream, not the code: WapuPay staging marks expiry
asynchronously — the tentative still reports FUNDING_ISSUED more than 6 minutes after
expires_at. Callers must not assume EXPIRED appears at the deadline; treat
expires_at as "do not pay after this", not as "status flips at this". The local flag
logic (order_is_final / order_is_failed mapping EXPIRED → final+failed) is
unit-covered; re-check the live flip later with
order_status 1e1df359-b959-4e7f-84ce-3e689aaf542d if desired.

9. Guard paths — unit-test proof (cannot be triggered against a real API)

Findings 2–5 fire only when WapuPay violates its own contract (flipped rail, wrong asset
id, fractional/zero/negative/string sats). The real staging API answers correctly, so
these are proven by the tests added with the fixes — all passing on this branch:

test_fund_order_thin_record_infers_lbtc_rail_from_asset_id            PASSED
test_fund_order_unknown_rail_refuses_to_name_a_send_amount            PASSED
test_from_dict_infers_lbtc_rail_before_the_legacy_scrub               PASSED
test_fund_order_rejects_rail_flip_on_existing_record                  PASSED
test_order_status_rejects_rail_flip_on_existing_record                PASSED
test_create_order_annotates_record_when_funding_response_is_rejected  PASSED
test_fund_order_annotates_stored_record_when_response_is_rejected     PASSED
test_fund_order_thin_record_rejects_echoed_rail_with_wrong_asset      PASSED
test_usdt_thin_fallback_names_the_base_units_field                    PASSED
test_create_order_rejects_rail_mismatch_from_wapupay                  PASSED
test_create_order_rejects_rail_flip_on_the_funding_response           PASSED
test_create_order_rejects_wrong_asset_for_lbtc_rail                   PASSED
test_create_order_rejects_wrong_asset_for_usdt_rail                   PASSED
test_total_amount_sats_must_be_a_positive_integer                     PASSED
14 passed, 86 deselected in 0.39s

Full suite on the branch: 1166 passed, 42 skipped.

10. Full re-run with a real alias

The first run used the synthetic alias aqua.test.stage (the sandbox accepts any alias).
The whole live sequence was repeated with the user-provided real alias
matriz.menudo.saber — again never paid:

quote: {"exchange_rate": 1554.72, "fee": 0.13, "total_amount": 6.56,
        "usdt_amount": 6.43, "valid_cbu_alias": true}

create_order (USDT) -> tentative 2c1b9b30-0ec0-4d25-86e1-4ebd3e5302fa
  status: FUNDING_ISSUED | funded: True
  funding_currency: USDT | total_amount_usdt: 6.56 | base_units: 656000000 | sats: None
  asset_id: ce091c99…7bd2 (USDT policy constant)
  payout clause present ("ARS to matriz.menudo.saber"): True

create_order (LBTC) -> WapuPay request failed (400 POST):
  Direct-fiat tentative funding via LBTC on Liquid is not available at the moment.

thin-record recovery (record deleted, then fund_order):
  rail: USDT | base_units: 656000000 | sats: None
  instructions: "Send exactly 6.56 USDT (656000000 base units) on Liquid to lq1qqdtfw…"
reloaded record -> rail: USDT | base_units: 656000000

order_status -> FUNDING_ISSUED | is_final: False | warning: None | last_error: None
  expires_at: 2026-08-16T11:15:33Z

Same results as the synthetic-alias run on every check, now against a real recipient
alias. The L-BTC rail stays blocked upstream (identical 400).

Conclusion

Everything that can be exercised against the live staging API passes on the fixed branch,
in both the synthetic-alias and the real-alias (matriz.menudo.saber) runs: keyed reads,
USDT order creation with correct integer-base-unit instructions, the finding-1
thin-record recovery, reload round-trips, and status polling. The live expiry flip was
not observed — staging marks expiry with a multi-minute lag (§8), an upstream behavior
worth knowing, not a code defect.
The one open live gap is the L-BTC rail itself — currently disabled upstream on staging
(clean 400, no local side effects). The contract-violation guards (findings 2–5) are, by
nature, only provable with injected malformed responses and are green in the unit suite.

Follow-up when WapuPay re-enables LBTC on staging: repeat §3–§8 with
funding_method="LBTC" and verify pay_instructions quotes total_amount_sats sats with
asset_id=6f0279e9…526d, and total_funding_amount_base_units stays null.

…ency A thin cross-device record (fund_order/order_status with no local order) has funding_currency=None, so every denomination branch silently fell back to USDT semantics. If the funding response carried total_amount_usdt next to the L-BTC asset_id, pay_instructions paired a USDT-scale amount (~10^8x) with the L-BTC asset. On reload, from_dict then scrubbed the real total_amount_sats as legacy-USDT residue and kept the wrong base units. - Back-fill funding_currency from a known Liquid policy asset_id (LBTC_ASSET_ID -> LBTC, USDT_LIQUID_ASSET_ID -> USDT) in apply_tentative and in from_dict (before the legacy scrub, so real sats survive reload). - Gate the USDT pay_instructions branch on an explicit USDT rail. - When the rail stays unknown (unknown asset), refuse to name any send amount and point at order_status instead. Finding 1 (HIGH) of the PR #122 review report.

_assert_rail guarded only the funding_currency echo; asset_id — the field lw_send_asset actually spends by — was passed verbatim from WapuPay's response into pay_instructions. An L-BTC order whose funding response carried the USDT asset id told the caller to send the sat figure as USDT base units: the order never settles and funds leave in an unquoted asset. Both rails settle in a Liquid policy asset whose id is a global constant (LBTC_ASSET_ID / USDT_LIQUID_ASSET_ID), so the check is free: any other asset_id for a known rail is an upstream contract violation and raises, annotating the persisted record (funded=True) like the currency flip. Finding 2 (MEDIUM) of the PR #122 review report.

create_order raised via _assert_rail when WapuPay's echo contradicted the requested rail, but the re-issue path (fund_order) and the poll path (order_status) merged the response unchecked: a flipped echo silently re-denominated the stored record and fund_order then emitted instructions for a rail the user never chose. Both paths now run the same _assert_rail check against the rail stored before the merge (via a shared _assert_known_rail helper). Thin records with no stored rail still get the asset-consistency half of the check against the echoed rail. order_status is restructured so only the NETWORK failure degrades to the last-known-local warning — a money-contract violation in the response now raises instead of displaying. Finding 3 (LOW) of the PR #122 review report.

In create_order, the post-funding apply_tentative(funding) ran outside the try that records last_error. Its contract-violation raise (fractional total_amount_sats) left the persisted record CREATED with no last_error while funding existed upstream, and recovery via fund_order hit the same un-annotated raise forever. Both create_order and fund_order now annotate the record via a shared _annotate_rejected_response helper, mirroring _assert_rail(funded=True). The clean STORED record is annotated — the half-merged in-memory order is not saved, so a rejected response never leaves its contract-violating values on disk. Finding 4 (LOW) of the PR #122 review report.

The new strictness check on the L-BTC boundary handled only fractional floats: a zero or negative integer sailed through into pay_instructions ('Send exactly -25127 sats of L-BTC ...'), and a string-typed value would round-trip into storage uncoerced. The USDT rail already rejects non-positive totals inside usdt_to_base_units, so the L-BTC seam was asymmetrically weaker. One shared validation now requires a positive int (whole floats coerced, bool excluded) and raises the same contract-violation ValueError otherwise. This also covers the string-typed hardening noted in the review's finding 8. Finding 5 (LOW) of the PR #122 review report.

The fallback told a USDT payer to fetch total_amount_usdt — a decimal — and 'pay that exact amount with lw_send_asset', whose amount parameter is integer base units: following it literally underpays by ~10^8x (or errors on the non-integer). Name total_funding_amount_base_units instead — the field that is directly payable and that order_status re-derives on load — mirroring how the L-BTC half already names total_amount_sats. Finding 6 (LOW) of the PR #122 review report.

coelhogonzalo added 6 commits August 15, 2026 21:00

coelhogonzalo marked this pull request as ready for review August 16, 2026 12:56

coelhogonzalo requested review from andycreed0x and rocket-morgan August 16, 2026 12:56

coelhogonzalo mentioned this pull request Aug 16, 2026

Add L-BTC funding option to WapuPay direct-fiat orders #122

Open

3 tasks

Navigation Menu

Uh oh!

👀 Use LBTC on wapupay PR review - #130

👀 Use LBTC on wapupay PR review#130
coelhogonzalo wants to merge 6 commits into
use-lbtc-on-wapupayfrom
use-lbtc-on-wapupay-pr-review

coelhogonzalo commented Aug 16, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

coelhogonzalo commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Integration test summary with a real Argentinian alias

Result summary

Evidence

1. Public rates — both hosts reachable through the branch code

2. Keyed read-only calls

3. USDT order — created, funded, correct instructions

4. L-BTC order — upstream unavailable on staging (not a code failure)

5. Thin-record recovery (finding 1) — delete the record, re-fund

6. Reload round-trip

7. order_status on the rebuilt record

8. Expiry → terminal state (upstream behavior finding)

9. Guard paths — unit-test proof (cannot be triggered against a real API)

10. Full re-run with a real alias

Conclusion

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coelhogonzalo commented Aug 16, 2026 •

edited

Loading

7. `order_status` on the rebuilt record